-
-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improve RC workflow: Enhanced validation and error handling in Makefile #270
Conversation
Pull Request Test Coverage Report for Build 9271628389Details
💛 - Coveralls |
7e2e813
to
2345fb4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we can document the version calculation logic so that we can all easily review it, makefile syntax makes it a little hard to follow, what do you think?
@echo "[INFO] Fetching the latest version from PyPI." | ||
$(eval LATEST_VERSION=$(shell curl -sL "https://pypi.org/pypi/leverage/json" | jq -r ".releases | keys[]" | $(SORT) -V | tail -n 1)) | ||
@echo "[INFO] Latest version fetched from PyPI: $(LATEST_VERSION)" | ||
|
||
# Get the latest RC version from PyPI if it exists | ||
$(eval LATEST_RC_VERSION=$(shell curl -sL "https://pypi.org/pypi/leverage/json" | jq -r ".releases | keys[]" | grep "$(LATEST_VERSION)-rc" | $(SORT) -V | tail -n 1)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At the time of writing this comment:
$ curl -sL "https://pypi.org/pypi/leverage/json" | jq -r '.releases | keys[]' | sort -V | tail -n 2
1.12.3
1.12.3rc1
And so, after running these two statements
LATEST_VERSION=1.12.3rc1
LATEST_RC_VERSION=""
since rc
s currently do not use the dash separator. But if we ignore this, both variables would hold the same value.
Suppose we don't have 1.12.3rc1
as a released version, we would end up with something like
LATEST_VERSION=1.12.3
LATEST_RC_VERSION=1.12.2rc3
which would create various different issues down the line like attempting to release 1.12.3rc4
without the previous 3 existing (following the logic in lines 77 to 83).
Couldn't we rely only on LATEST_VERSION
?
if [ "$(PROVIDED_RC_VERSION)" -le "$(shell echo $(LATEST_RC_VERSION) | sed 's/.*-rc//')" ]; then \ | ||
echo "[INFO] Provided RC version is valid."; \ | ||
else \ | ||
echo "[ERROR] Provided RC version is invalid as it does not follow the latest RC version from PyPI. Exiting..."; \ | ||
exit 1; \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By accepting PROVIDED_RC_VERSION
s that are lesser or equal that the current latest rc version we will most likely clash with an already existing release, wouldn't we? also we wouldn't be able to progress in the rc
s version if I'm not mistaken.
I think rc
s should be always incrementing in version number since they are for testing purposes, what do you think about it?
As discussed with @angelofenoglio and @juanmatias we'll close this issue for the moment. If needed in the future we can reopen it. |
What?
bump-test-version
target in the Makefile to ensure proper validation of the provided release candidate (RC) versions.rc0
if no RC versions exist or incrementing the latest RC version from PyPI.Why?
References
Before release
Review the checklist here